Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Doctrine integration Fix #146, #152 #149

Merged
merged 1 commit into from
Apr 19, 2018

Conversation

sadikoff
Copy link
Contributor

@sadikoff sadikoff commented Apr 11, 2018

This will close #146

in progress...

@sadikoff sadikoff changed the title [WIP] Fix #146: working with table inheritance [WIP] Improve Doctrine integration Fix #146, #152 Apr 18, 2018
@sadikoff sadikoff changed the title [WIP] Improve Doctrine integration Fix #146, #152 Improve Doctrine integration Fix #146, #152 Apr 18, 2018
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments. This is really great work!

$this->fileManager = $fileManager;
$this->generator = $generator;
$this->projectDirectory = $projectDirectory;
$this->overwrite = $overwrite;
$this->metadataFactory = new DoctrineMetadataFactory($this->doctrineRegistry);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DoctrineMetadataFactory calls should be deleted entirely now, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and DoctrineMetadataFactory class too I think

*
* @throws \Exception
*/
public function createDoctrineDetails(string $entityClassName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use a return type here instead of @returns

return $entities;
}

public function getMetadata(string $classOrNamespace = null, bool $disconnected = false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a return type here too

/**
* @ORM\Column(type="string", length=255)
*/
private $ingridients;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: ingredients :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops :)

@weaverryan
Copy link
Member

Very nice, very careful work. Thank you @sadikoff!

@weaverryan weaverryan merged commit 7b8ac42 into symfony:master Apr 19, 2018
weaverryan added a commit that referenced this pull request Apr 19, 2018
This PR was squashed before being merged into the 1.0-dev branch (closes #149).

Discussion
----------

Improve Doctrine integration Fix #146, #152

This will close #146

in progress...

Commits
-------

7b8ac42 Improve Doctrine integration Fix #146, #152
@nicodemuz
Copy link

@weaverryan I think this introduced a bug:

Undefined class constant 'ONE_TO_MANY'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make:* commands fail on entities using Single Table Inheritance
3 participants